feat: include TTY and tmux pane in session list#90
Conversation
Add tty and tmuxPane fields to HunkSessionRegistration and ListedSession so `hunk session list` and `hunk session get` can show which terminal a session belongs to. This makes it easier to identify sessions when multiple are running. Closes #89 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
@greptileai review |
Greptile SummaryThis PR enriches session registration with terminal location metadata — TTY path (resolved via Confidence Score: 4/5Safe to merge after addressing the locale-sensitive stdout check in The feature is additive and backward-compatible, test coverage is comprehensive, and the architecture is well-designed. One P1 concern: the
Important Files Changed
Sequence DiagramsequenceDiagram
participant TUI as Hunk TUI process
participant SR as sessionRegistration.ts
participant STM as sessionTerminalMetadata.ts
participant tty as tty command
participant DS as HunkDaemonState
participant CLI as hunk session list/get
TUI->>SR: createSessionRegistration(bootstrap)
SR->>SR: ttyname() — check process.stdin.isTTY
SR->>tty: spawnSync("tty")
tty-->>SR: /dev/ttys003
SR->>STM: resolveSessionTerminalMetadata({ tty, env })
STM->>STM: read TMUX_PANE, ITERM_SESSION_ID, TERM_SESSION_ID, TERM_PROGRAM
STM-->>SR: SessionTerminalMetadata { program, locations[] }
SR-->>DS: HunkSessionRegistration (with terminal field)
DS->>DS: registerSession — stores terminal in ListedSession
CLI->>DS: listSessions() / getSession()
DS-->>CLI: ListedSession[] (with terminal)
CLI->>CLI: formatTerminalLines() → text output
Reviews (1): Last reviewed commit: "refactor: generalize session terminal me..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| try { | ||
| const result = spawnSync("tty", [], { stdio: ["inherit", "pipe", "pipe"] }); | ||
| const name = result.stdout?.toString().trim(); | ||
| return name && !name.startsWith("not a tty") ? name : undefined; | ||
| } catch { | ||
| return undefined; |
There was a problem hiding this comment.
The current guard against a failed tty invocation checks whether stdout starts with "not a tty", which is an English-locale assumption. On non-English systems the tty command may emit a localised error string that passes through this check, potentially storing a garbage path. Checking result.status === 0 is locale-independent and more robust:
| try { | |
| const result = spawnSync("tty", [], { stdio: ["inherit", "pipe", "pipe"] }); | |
| const name = result.stdout?.toString().trim(); | |
| return name && !name.startsWith("not a tty") ? name : undefined; | |
| } catch { | |
| return undefined; | |
| const result = spawnSync("tty", [], { stdio: ["inherit", "pipe", "pipe"] }); | |
| if (result.status !== 0) { | |
| return undefined; | |
| } | |
| const name = result.stdout?.toString().trim(); | |
| return name || undefined; |
| function resolveSessionTerminal(session: ListedSession) { | ||
| return session.terminal; | ||
| } |
There was a problem hiding this comment.
resolveSessionTerminal returns session.terminal unchanged. The two call sites can access the field directly, avoiding an unnecessary indirection that suggests the function performs some computation. Replace both usages with session.terminal directly.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| return { | ||
| windowId: match.groups.window, | ||
| tabId: match.groups.tab, | ||
| paneId: match.groups.pane, | ||
| } satisfies Pick<SessionTerminalLocation, "windowId" | "tabId" | "paneId">; |
There was a problem hiding this comment.
undefined property
When the hierarchical ID has no pane (e.g. "w1t2:ABC"), match.groups.pane is undefined. Spreading { windowId: "1", tabId: "2", paneId: undefined } into the location object creates an explicit paneId: undefined key. While JSON serialisation drops it, it may surprise code that iterates the object's own keys. Consider filtering out undefined entries before returning:
| return { | |
| windowId: match.groups.window, | |
| tabId: match.groups.tab, | |
| paneId: match.groups.pane, | |
| } satisfies Pick<SessionTerminalLocation, "windowId" | "tabId" | "paneId">; | |
| return Object.fromEntries( | |
| Object.entries({ | |
| windowId: match.groups.window, | |
| tabId: match.groups.tab, | |
| paneId: match.groups.pane, | |
| }).filter(([, v]) => v !== undefined), | |
| ) as Pick<SessionTerminalLocation, "windowId" | "tabId" | "paneId">; |
1 participant
Summary
ttyandtmuxPanefields to session registration sohunk session listandhunk session getshow which terminal each session belongs tottycommand at registration time; tmux pane read fromTMUX_PANEenv varCloses #89
Test plan
🤖 Generated with Claude Code